Skip to content

go/store/nbs: Fix possible data loss from Dolt CLI utilization when executing against a running server that was in the process of writing to its journal file.#10341

Merged
reltuk merged 2 commits intomainfrom
aaron/fix-10331-no-truncate-on-read-only-journal-open
Jan 21, 2026
Merged

go/store/nbs: Fix possible data loss from Dolt CLI utilization when executing against a running server that was in the process of writing to its journal file.#10341
reltuk merged 2 commits intomainfrom
aaron/fix-10331-no-truncate-on-read-only-journal-open

Conversation

@reltuk
Copy link
Copy Markdown
Contributor

@reltuk reltuk commented Jan 20, 2026

Since Dolt v1.78.5, Dolt has truncated the journal file to the latest successfully loaded record when it successfully loads a database. This is the correct behavior when the CLI is operating as the exclusive writer to the database. However, Dolt also has a mode where it can load the database in read-only mode. Due to a bug, Dolt was also truncating the journal file when it was operating in this mode. The end result was the running something like dolt sql -r csv -q ... against a Dolt database that was running a sql-server and was currently accepting writes could incorrectly truncate the journal file. The Dolt sql-server process would go ahead writing into the journal at its previously extended offset and the space between the truncation and the next write offset would be 0 filled by the operating system. Attempting to load the database later or accessing the chunks located at that corrupted portion of the journal file would result in checksum errors or failure to load messages.

This change correctly updates Dolt to only Truncate the journal file when we are loading it in read-write mode.

…xecuting against a running server that was in the process of writing to its journal file.

Since Dolt v1.78.5, Dolt has truncated the journal file to the latest successfully loaded record when it successfully loads a database. This is the correct behavior when the CLI is operating as the exclusive writer to the database. However, Dolt also has a mode where it can load the database in read-only mode. Due to a bug, Dolt was also truncating the journal file when it was operating in this mode. The end result was the running something like `dolt sql -r csv -q ...` against a Dolt database that was running a sql-server and was currently accepting writes could incorrectly truncate the journal file. The Dolt sql-server process would go ahead writing into the journal at its previously extended offset and the space between the truncation and the next write offset would be 0 filled by the operating system. Attempting to load the database later or accessing the chunks located at that corrupted portion of the journal file would result in checksum errors or failure to load messages.

This change correctly updates Dolt to only Truncate the journal file when we are loading it in read-write mode.
@reltuk reltuk requested a review from macneale4 January 20, 2026 22:53
Copy link
Copy Markdown
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about a test? Fine to ship without another review


var recoverErr error
n, err := processJournalRecords(ctx, bytes.NewReader(journal), 0, check, func(e error) { recoverErr = e })
n, err := processJournalRecords(ctx, bytes.NewReader(journal), true, 0, check, func(e error) { recoverErr = e })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about a test where we pass false and watch it do the right thing?

@coffeegoddd
Copy link
Copy Markdown
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
ffef6e9 ok 5937471
version total_tests
ffef6e9 5937471
correctness_percentage
100.0

@coffeegoddd
Copy link
Copy Markdown
Contributor

@reltuk DOLT

comparing_percentages
100.000000 to 100.000000
version result total
bbb46c2 ok 5937471
version total_tests
bbb46c2 5937471
correctness_percentage
100.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants